London | ITP-Jan-26 | Mohsen Zamani | Sprint 3 | stretch#939
London | ITP-Jan-26 | Mohsen Zamani | Sprint 3 | stretch#939mohsenzamanist wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
mohsenzamanist
changed the title
mohsenzamanist
changed the title
mohsenzamanist
added
the
Needs Review
Sprint-3/4-stretch/card-validator.js
Outdated
| !cardNumArray.every( | ||
| (num) => num.charCodeAt(0) >= 48 && num.charCodeAt(0) <= 57 | ||
| ) |
There was a problem hiding this comment.
Note: We could also check if num is a digit as num >= '0' && num <= '9'.
Sprint-3/4-stretch/card-validator.js
Outdated
| const count = cardNumArray.reduce((acc, curr) => { | ||
| acc[curr] = acc[curr] ? acc[curr] + 1 : 1; | ||
| return acc; | ||
| }, {}); | ||
| if (Object.keys(count).length < 2) return false; |
There was a problem hiding this comment.
This works.
Challenge: Implement a simpler and more efficient way to check distinct digit count with Set.
| @@ -1,6 +1,12 @@ | |||
| const previousPasswords = ["5B43n21"]; | |||
| function passwordValidator(password) { | |||
There was a problem hiding this comment.
If we design the function as
function passwordValidator(password, previousPasswords=[]) {
...
}
we could let the caller specify what the previous passwords are.
| @@ -1,6 +1,12 @@ | |||
| const previousPasswords = ["5B43n21"]; | |||
There was a problem hiding this comment.
An invalid password is not a good candidate for a previous password.
| } | ||
| ); No newline at end of file | ||
| // Arrange | ||
| const password = "A1b2"; |
There was a problem hiding this comment.
The function can return false for multiple reasons.
To test a specific reason, choose an input that satisfies all other conditions except the one you're targeting. This way, if the function returns false, you can confidently attribute it to that specific condition.
For example, the function might return false for "A1b2" not only because it is fewer than 5 characters, but also because it lacks a special letter.
As a result, we can't be certain that the function correctly handles the case of passwords shorter than 5 characters, since multiple conditions are being violated simultaneously.
| test("password contains at least one uppercase English letter", () => { | ||
| // Arrange | ||
| const password = "1a2345"; | ||
| // Act | ||
| const result = isValidPassword(password); | ||
| // Assert | ||
| expect(result).toEqual(false); | ||
| }); | ||
|
|
||
| test("password contains at least one uppercase English letter", () => { | ||
| // Arrange | ||
| const password = "1B2345"; | ||
| // Act | ||
| const result = isValidPassword(password); | ||
| // Assert | ||
| expect(result).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
- These two tests have the same description but they are testing two different things.
There was a problem hiding this comment.
Could you make the test descriptions more detailed and descriptive? That way, if a test fails, the person implementing the function can quickly understand what went wrong and why.
Right now, for example, "password contains at least one number (0-9)" does not quite indicate what the test is testing. (Did you mean to say "not containing")
cjyuan
added
Reviewed
mohsenzamanist
added
Needs Review
2 participants
Learners, PR Template
Self checklist
Changelist
Comelete exercises in directory called 4-stretch
Questions